🤖 Kimi Code Review

Review Summary
Good practice change to pin floating dependencies for CI stability. One minor documentation error to fix.

Issues Found

1. Incorrect date in comment (.github/workflows/pr-main_l1.yaml, line 238)

# Source: ethereum/execution-specs devnets/bal/3 @ 2026-04-14

The date 2026-04-14 appears to be in the future. If this is a typo, correct it to the actual date (likely 2025-04-14 or similar).

2. Significant CI policy change
Removing optional: true (line 241) makes Amsterdam fork tests mandatory for PR merges. While pinning the EELS commit mitigates upstream breakage, ensure the team is ready to block merges on these tests if the Amsterdam spec is still evolving. The logic is sound (pinning enables requiring), but verify this aligns with current development priorities.

Positive Aspects

• Security/Reproducibility: Pinning to commit hash 5c6e20abf3586f52d9e58393203ca07f2d0151fe instead of branch devnets/bal/3 prevents supply chain attacks and ensures deterministic CI runs.
• Consistency: Both workflow files updated to use the same pinned commit.
• Documentation: Updated comments explain why the pin exists and when to bump it.

Recommendation: Fix the date typo, then merge.

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

Greptile Summary

This PR pins the EELS dependency in both the PR and daily Hive CI workflows to a specific commit hash (5c6e20abf3586f52d9e58393203ca07f2d0151fe from devnets/bal/3 @ 2026-04-14) instead of the mutable branch name, and promotes the Amsterdam Consume Engine job from optional to required.

The commit hash is consistent across both workflow files, and the buildarg splitting mechanism in pr-main_l1.yaml (line 285: for arg in $SIM_BUILDARG) correctly handles the space-separated fixtures=... branch=... format unchanged from before.

Confidence Score: 5/5

Safe to merge — straightforward CI hardening with no logic changes.

Both changes are mechanical: swapping a mutable branch name for a pinned commit hash (consistently across both workflow files) and removing optional: true. No application logic is touched, the buildarg parsing mechanism is unchanged, and the commit hash is identical in both files.

No files require special attention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[PR / Daily Schedule Triggered] --> B{Workflow}
    B --> C[pr-main_l1.yaml]
    B --> D[daily_hive_report.yaml]

    C --> E[Matrix: Consume Engine Amsterdam]
    E --> F["buildarg: fixtures=bal@v5.6.1\nbranch=5c6e20abf..."]
    F --> G[Hive splits on whitespace\nfor arg in SIM_BUILDARG]
    G --> H["--sim.buildarg fixtures=...\n--sim.buildarg branch=5c6e20abf..."]
    H --> I[Run Hive Simulation]
    I --> J[Required check ✅\noptional: true REMOVED]

    D --> K{SIM_LIMIT contains fork_Amsterdam?}
    K -- Yes --> L["branch=5c6e20abf... pinned commit"]
    K -- No --> M["branch=forks/osaka"]
    L --> N[Run daily Hive report]
    M --> N

_{Reviews (1): Last reviewed commit: "ci(l1): add original branch comment to d..." | Re-trigger Greptile}

🤖 Codex Code Review

1. Medium: Removing optional: true from Consume Engine Amsterdam changes it from informational to merge-blocking. run-hive is configured with continue-on-error: ${{ matrix.optional || false }} at pr-main_l1.yaml, so the change at pr-main_l1.yaml through pr-main_l1.yaml is a behavioral gate change, not just a pin. The new comment only explains pinning to avoid upstream CI breakage; it does not justify promoting an Amsterdam test suite that was previously marked unstable into a required check. If the intent is only to freeze upstream inputs, optional: true should stay until the suite is explicitly ready to gate PRs.

No other correctness, security, or performance concerns stood out in this workflow-only diff.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt